Skip to content

central_systest_blobs: add proof tx to test privacy feature#14006

Open
dorimedini-starkware wants to merge 1 commit into
05-05-starknet_api_add_and_use_stateroots_emptyfrom
05-09-central_systest_blobs_add_proof_tx_to_test_privacy_feature
Open

central_systest_blobs: add proof tx to test privacy feature#14006
dorimedini-starkware wants to merge 1 commit into
05-05-starknet_api_add_and_use_stateroots_emptyfrom
05-09-central_systest_blobs_add_proof_tx_to_test_privacy_feature

Conversation

@dorimedini-starkware

@dorimedini-starkware dorimedini-starkware commented May 9, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

This was referenced May 9, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@reviewable-StarkWare

reviewable-StarkWare commented May 9, 2026

Copy link
Copy Markdown

This change is Reviewable

This was referenced May 9, 2026
Merged
Merged
This was referenced May 9, 2026
Merged
Merged
This was referenced May 9, 2026
Merged
Merged
@dorimedini-starkware dorimedini-starkware self-assigned this May 9, 2026
This was referenced May 9, 2026
Merged
Merged
@github-actions

github-actions Bot commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Artifacts upload workflows:

This was referenced May 9, 2026
Merged
Merged
@dorimedini-starkware dorimedini-starkware marked this pull request as ready for review May 9, 2026 14:12
@dorimedini-starkware Graphite App

dorimedini-starkware commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@cursor

cursor Bot commented May 9, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Low Risk
Low risk: changes are confined to test data generation and a test utility macro tweak (optional trailing comma), with no production execution paths altered.

Overview
Adds coverage for proof-carrying transactions in central_systest_blobs by generating an operator invoke that includes valid virtual SNOS proof_facts (derived from a retrospective block hash and PROGRAM_HASHES.virtual_os).

Updates the expected regression artifacts (blob generation id and preconfirmed_block.json) to reflect the additional block/transaction and resulting hash/nonce/storage diffs, and relaxes the starknet_api::proof_facts! macro to accept a trailing comma.

Reviewed by Cursor Bugbot for commit dc5d215. Bugbot is set up for automated code reviews on this repo. Configure here.

@dorimedini-starkware dorimedini-starkware force-pushed the 05-05-starknet_api_add_and_use_stateroots_empty branch from 80f2783 to 7741718 Compare May 9, 2026 18:52
@dorimedini-starkware dorimedini-starkware force-pushed the 05-09-central_systest_blobs_add_proof_tx_to_test_privacy_feature branch from 69e30f0 to 539ab80 Compare May 9, 2026 18:52
nimrod-starkware
nimrod-starkware requested changes May 10, 2026
View reviewed changes

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nimrod-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on dorimedini-starkware and yoavGrs).

crates/central_systest_blobs/src/cende_blob_regression_test.rs line 699 at r1 (raw file):

            .allowed_virtual_os_program_hashes
            .first()
            .expect("No allowed virtual OS program hashes in versioned constants");

Is there a constant for that?
I don't like the usage of first here, what if the order of vec changes? (currently it has only one entry)

Code quote:

        let program_hash = *block_context
            .versioned_constants()
            .os_constants
            .allowed_virtual_os_program_hashes
            .first()
            .expect("No allowed virtual OS program hashes in versioned constants");

nimrod-starkware
nimrod-starkware requested changes May 10, 2026
View reviewed changes

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nimrod-starkware made 1 comment.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on dorimedini-starkware and yoavGrs).

crates/central_systest_blobs/src/cende_blob_regression_test.rs line 699 at r1 (raw file):

Previously, nimrod-starkware wrote…

Is there a constant for that?
I don't like the usage of first here, what if the order of vec changes? (currently it has only one entry)

I think you can use ProgramHashes.virtual_os

@dorimedini-starkware dorimedini-starkware force-pushed the 05-09-central_systest_blobs_add_proof_tx_to_test_privacy_feature branch from 539ab80 to 8737d63 Compare May 10, 2026 06:42
@dorimedini-starkware dorimedini-starkware force-pushed the 05-05-starknet_api_add_and_use_stateroots_empty branch from 7741718 to 884af7d Compare May 10, 2026 06:42
dorimedini-starkware
dorimedini-starkware commented May 10, 2026
View reviewed changes

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dorimedini-starkware made 1 comment.
Reviewable status: 3 of 6 files reviewed, 1 unresolved discussion (waiting on nimrod-starkware and yoavGrs).

crates/central_systest_blobs/src/cende_blob_regression_test.rs line 699 at r1 (raw file):

Previously, nimrod-starkware wrote…

I think you can use ProgramHashes.virtual_os

Done.

nimrod-starkware
nimrod-starkware approved these changes May 10, 2026
View reviewed changes

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@nimrod-starkware reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on yoavGrs).

@dorimedini-starkware dorimedini-starkware force-pushed the 05-05-starknet_api_add_and_use_stateroots_empty branch from 884af7d to 3b9a787 Compare May 10, 2026 07:59
@dorimedini-starkware dorimedini-starkware force-pushed the 05-09-central_systest_blobs_add_proof_tx_to_test_privacy_feature branch 2 times, most recently from 5c2fb4f to de29e87 Compare May 10, 2026 09:54
@dorimedini-starkware dorimedini-starkware force-pushed the 05-05-starknet_api_add_and_use_stateroots_empty branch from 3b9a787 to 3d97fd4 Compare May 10, 2026 09:54
@dorimedini-starkware dorimedini-starkware force-pushed the 05-05-starknet_api_add_and_use_stateroots_empty branch from 3d97fd4 to fa76e6e Compare May 10, 2026 11:21
@dorimedini-starkware dorimedini-starkware force-pushed the 05-09-central_systest_blobs_add_proof_tx_to_test_privacy_feature branch from de29e87 to dc5d215 Compare May 10, 2026 11:21
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale.
This PR will be closed and locked in 7 days if no further activity occurs.
Thank you for your contributions!

@github-actions github-actions Bot added stale and removed stale labels Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nimrod-starkware nimrod-starkware nimrod-starkware approved these changes
@yoavGrs yoavGrs Awaiting requested review from yoavGrs

Assignees

@dorimedini-starkware dorimedini-starkware

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dorimedini-starkware @reviewable-StarkWare @nimrod-starkware